fix(ingress): harden public platform exposure (#108)#109
fix(ingress): harden public platform exposure (#108)#109Agent-Hellboy wants to merge 1 commit intomainfrom
Conversation
Addresses items 1, 2, 4, and 6 of the platform.mcpruntime.org security audit. Items 3 (unauth MCP tool execution) and 5 (public /api/runtime/servers) are intentionally not in this change. - UI service now sets baseline security headers (CSP with frame-ancestors 'none', X-Content-Type-Options, Referrer-Policy, Permissions-Policy, Cache-Control no-store on /api and /auth) and HSTS only when X-Forwarded-Proto is https. - New httpsRedirectMiddleware in the UI 308s HTTP -> HTTPS based on X-Forwarded-Proto. Behavior is gated by UI_REQUIRE_HTTPS (auto/true/false); auto skips localhost / IP-only hosts so the Kind dev stack is unaffected. - Platform Ingress no longer exposes /grafana or /prometheus on the public host (those tools have no built-in auth in this stack; operators should reach them via port-forward). - When TLS is configured, an additional HTTP-only Ingress is emitted for the same host so plain http://platform.<domain>/ resolves to the UI (which then redirects), shadowing the host-less dev gateway Ingress that previously answered HTTP with 200. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 57.15% 57.21% +0.05%
==========================================
Files 59 59
Lines 10448 10462 +14
==========================================
+ Hits 5972 5986 +14
Misses 3905 3905
Partials 571 571
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request improves platform security and routing by restricting public access to Grafana and Prometheus and implementing automated HTTP-to-HTTPS redirection. It introduces middleware to manage security headers, including Content Security Policy (CSP), HSTS, and Cache-Control. Review feedback recommends further hardening the security configuration by refactoring the CSP to eliminate 'unsafe-inline', opting out of FLoC tracking via the Permissions-Policy header, and expanding Cache-Control directives for sensitive API and authentication endpoints.
| // Google Sign-In needs accounts.google.com for scripts/iframes/connect. | ||
| h.Set("Content-Security-Policy", | ||
| "default-src 'self'; "+ | ||
| "script-src 'self' 'unsafe-inline' https://accounts.google.com https://apis.google.com; "+ |
There was a problem hiding this comment.
The use of 'unsafe-inline' in the script-src directive of the Content Security Policy (CSP) significantly weakens the protection against Cross-Site Scripting (XSS) attacks. While it may be necessary for some legacy or third-party scripts, it is highly recommended to refactor the frontend to use nonces or hashes for inline scripts, or move them to external files, to allow for a more restrictive policy.
| h := w.Header() | ||
| h.Set("X-Content-Type-Options", "nosniff") | ||
| h.Set("Referrer-Policy", "strict-origin-when-cross-origin") | ||
| h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=(), payment=(), usb=()") |
| } | ||
| path := r.URL.Path | ||
| if strings.HasPrefix(path, "/api") || strings.HasPrefix(path, "/auth/") { | ||
| h.Set("Cache-Control", "no-store") |
There was a problem hiding this comment.
For maximum security on sensitive endpoints like /api and /auth/, consider expanding the Cache-Control header to include no-cache and must-revalidate (e.g., no-store, no-cache, must-revalidate). While no-store is the strongest directive, adding the others ensures that even if a browser or intermediary proxy doesn't fully respect no-store, the data is not served from cache without revalidation.
| h.Set("Cache-Control", "no-store") | |
| h.Set("Cache-Control", "no-store, no-cache, must-revalidate") |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1146e9c8c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return true | ||
| } | ||
| // No proxy header. Only redirect in forced mode for non-local hosts. | ||
| return strings.EqualFold(strings.TrimSpace(mode), "true") && !isLocalHost(r.Host) |
There was a problem hiding this comment.
Treat on/1/yes as forced HTTPS mode
The forced-mode aliases declared in shouldRedirectToHTTPS are not handled consistently when X-Forwarded-Proto is missing: "on", "1", and "yes" enter the forced-mode branch in the switch, but the fallback path only checks mode == "true". In deployments where the upstream proxy does not set X-Forwarded-Proto, setting UI_REQUIRE_HTTPS=on (or 1/yes) silently disables the intended redirect and allows plain-HTTP responses for public hosts.
Useful? React with 👍 / 👎.
Addresses items 1, 2, 4, and 6 of the platform.mcpruntime.org security audit. Items 3 (unauth MCP tool execution) and 5 (public /api/runtime/servers) are intentionally not in this change.